Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make some ops new be const fn #5027

Closed
wants to merge 3 commits into from

Conversation

yjhmelody
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

make some new be const fn

Are there any user-facing changes?

No

@tisonkun
Copy link
Member

tisonkun commented Oct 8, 2024

We may not randomly mark fn new as const fn just because it can be. This is a one-way door that revoke the const modifier would be a breaking change.

Please explain why those fn SHOULD BE constant function.

@yjhmelody
Copy link
Contributor Author

We may not randomly mark fn new as const fn just because it can be. This is a one-way door that revoke the const modifier would be a breaking change.

Please explain why those fn SHOULD BE constant function.

But why will change a const function to a non-const function back when there is no breaking change about this function?

There is no reason here, just to make it more general, I will close this PR

@yjhmelody yjhmelody closed this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants